Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loosen tolerance in a few expm() tests #38685

Merged
merged 4 commits into from
Sep 29, 2024
Merged

Conversation

orlitzky
Copy link
Contributor

SciPy upstream just merged scipy/scipy#21553 which rewrites the matrix exponential in C. I merged this PR to work around some other bug, and in the process noticed that a few tolerances will need to be loosened in our test suite.

Change 1r-14 to 5e-14 in the following two examples. This is likely
required because expm() in scipy was rewritten in C.

Example 1:

  File "src/sage/matrix/matrix_double_dense.pyx", line 3666, in
  sage.matrix.matrix_double_dense.Matrix_double_dense.exp
  Failed example:
    A.exp()  # tol 1e-14
  Expected:
    [51.968956198705044  74.73656456700327]
    [112.10484685050491 164.07380304920997]
  Got:
    [51.968956198707595    74.736564567007]
    [112.10484685051048 164.07380304921807]
  Tolerance exceeded in 4 of 4:
    51.968956198705044 vs 51.968956198707595, tolerance 5e-14 > 1e-14
    74.73656456700327 vs 74.736564567007, tolerance 5e-14 > 1e-14
    112.10484685050491 vs 112.10484685051048, tolerance 5e-14 > 1e-14
    164.07380304920997 vs 164.07380304921807, tolerance 5e-14 > 1e-14

Example 2:

  File "src/sage/matrix/matrix_double_dense.pyx", line 3679, in
  sage.matrix.matrix_double_dense.Matrix_double_dense.exp
  Failed example:
    A.exp()   # tol 1e-14
  Expected:
    [51.968956198705044  74.73656456700327]
    [112.10484685050491 164.07380304920997]
Got:
    [51.968956198707595    74.736564567007]
    [112.10484685051048 164.07380304921807]
  Tolerance exceeded in 4 of 4:
    51.968956198705044 vs 51.968956198707595, tolerance 5e-14 > 1e-14
    74.73656456700327 vs 74.736564567007, tolerance 5e-14 > 1e-14
    112.10484685050491 vs 112.10484685051048, tolerance 5e-14 > 1e-14
    164.07380304920997 vs 164.07380304921807, tolerance 5e-14 > 1e-14
Change 1e-14 to 6e-14 in the following example. This is likely
necessary because expm() in scipy was rewritten in C.

  File "src/sage/matrix/matrix2.pyx", line 15903,
  in sage.matrix.matrix2.Matrix.exp
  Failed example:
    a.change_ring(RDF).exp()  # rel tol 1e-14
  Expected:
    [42748127.31532951 7368259.244159399]
    [234538976.1381042 40426191.45156228]
  Got:
    [ 42748127.31533201  7368259.244159831]
    [234538976.13811788  40426191.45156465]
  Tolerance exceeded in 4 of 4:
    42748127.31532951 vs 42748127.31533201, tolerance 6e-14 > 1e-14
    7368259.244159399 vs 7368259.244159831, tolerance 6e-14 > 1e-14
    234538976.1381042 vs 234538976.13811788, tolerance 6e-14 > 1e-14
    40426191.45156228 vs 40426191.45156465, tolerance 6e-14 > 1e-14
In the following example, we now use 5e-14 instead of 1e-14. This is
likely due to scipy's expm() being rewritten in C:

  File "src/sage/symbolic/constants_c_impl.pxi", line 162, in
  sage.symbolic.constants_c_impl.pxi.E.__pow__
  Failed example:
    e^A  # rel tol 1e-14
  Expected:
    [51.968956198705044  74.73656456700327]
    [112.10484685050491 164.07380304920997]
  Got:
    [51.968956198707595    74.736564567007]
    [112.10484685051048 164.07380304921807]
 Tolerance exceeded in 4 of 4:
    51.968956198705044 vs 51.968956198707595, tolerance 5e-14 > 1e-14
    74.73656456700327 vs 74.736564567007, tolerance 5e-14 > 1e-14
    112.10484685050491 vs 112.10484685051048, tolerance 5e-14 > 1e-14
    164.07380304920997 vs 164.07380304921807, tolerance 5e-14 > 1e-14
Copy link

github-actions bot commented Sep 20, 2024

Documentation preview for this PR (built with commit a727c17; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. reasonable.

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 27, 2024
    
SciPy upstream just merged scipy/scipy#21553
which rewrites the matrix exponential in C. I merged this PR to work
around some other bug, and in the process noticed that a few tolerances
will need to be loosened in our test suite.
    
URL: sagemath#38685
Reported by: Michael Orlitzky
Reviewer(s): Kwankyu Lee
@orlitzky
Copy link
Contributor Author

thank you

@vbraun vbraun merged commit 9f791ae into sagemath:develop Sep 29, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants